feat: sanitize rich clipboard HTML - #65
Conversation
📝 WalkthroughWalkthroughSafeClipboard가 리치 HTML 클립보드를 allowlist 기반으로 정제하고, 제한 초과·설정·DOM 오류를 비공개 오류로 처리합니다. standalone 및 협업형 편집기가 이를 공유 확장으로 사용합니다. 관련 문서와 테스트, CI 실행 환경 및 checkout 보안 설정도 추가했습니다. ChangesSafeClipboard 리치 클립보드 정제
CI 실행 및 checkout 보안 강화
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Clipboard
participant CwlEditor
participant buildExtensions
participant SafeClipboard
participant TipTap
Clipboard->>CwlEditor: text/html 붙여넣기
CwlEditor->>buildExtensions: clipboard 설정 및 오류 콜백 전달
buildExtensions->>SafeClipboard: SafeClipboard 등록
SafeClipboard->>SafeClipboard: HTML 제한 및 allowlist 재구성
SafeClipboard->>TipTap: 정제 HTML 또는 빈 문자열 전달
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head finding (valid, blocking Draft→Ready): the standalone/collaborative integration bypasses the sanitizer's fail-closed ClipboardConfig validation. buildExtensions() dereferences clipboard.maxHtmlBytes, maxNodes, and maxDepth before sanitizeRichClipboardHtml() runs, then constructs a fresh known-key object. As a result, an accessor/proxy configuration can execute or throw during editor construction, and unknown/symbol/non-enumerable configuration keys are silently discarded rather than producing the documented redacted invalid_configuration error on rich paste. This contradicts docs/clipboard-security.md (“Invalid configuration fails closed when rich HTML is pasted”) and the direct sanitizer contract.
Please fix test-first without weakening the current direct API: add a regression proving buildExtensions()/both React surfaces do not evaluate nested accessors at construction, preserve the original config object into SafeClipboard, and return '' plus one redacted invalid_configuration callback on paste. A bounded option such as config?: ClipboardConfig may coexist with the current direct numeric extension options; the transform should validate the preserved config at paste time. Keep callback liveness, SSR import safety, and 100% statement/branch/function/line coverage.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head CI triage for 26cd91f53d97a6897b3dc74753d7a67ef21750ea:
-
Valid implementation failure:
mso-hide: allsurvives becauseCSSStyleDeclaration.getPropertyValue('mso-hide')drops/does not expose the proprietary declaration in jsdom (and cannot be relied on uniformly across engines). Inspect the rawstyleattribute without executing anything, parse bounded declarations, and drop the subtree when an exact case-insensitivemso-hide: alldeclaration is present. Keep ordinary style attributes discarded from output and add variants for whitespace, case,!important, CSS comments, and misleading values. -
Incorrect assertion:
expect(sanitized).not.toMatch(/section|font/i)necessarily fails because the same test explicitly requires preserving visible textfont text. Parse the sanitized fragment and assertquerySelector('section, font') === nullwhile retainingfont text; do not weaken the intended visible-text preservation.
The earlier blocking integration finding remains unresolved: buildExtensions() still dereferences and reconstructs ClipboardConfig, bypassing accessor/unknown-key fail-closed validation on both React surfaces. Address all three issues test-first before moving Draft→Ready.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Additional exact-head standards/security findings for a1c952e7124e7fb37e6ff7f5efada756c24cb93f:
-
Invalid/nonexistent standards citation:
docs/doctoring/safe-rich-clipboard.mdcites a “W3C Working Draft dated 24 June 2026” andhttps://www.w3.org/TR/2026/WD-clipboard-apis-20260624/. The official current W3C TR page and publication history identify the latest published Working Draft as 24 November 2025 (https://www.w3.org/TR/clipboard-apis/,https://www.w3.org/standards/history/clipboard-apis/). Replace the nonexistent date/version with the current official publication, preserve the work-in-progress claim boundary, and add a deterministic documentation assertion so a future fabricated dated URL cannot recur. -
Paste-transform ordering is not yet defended: current TipTap documentation states that extension
transformPastedHTMLhooks are chained by extension priority, with higher priority running first and each later transform receiving the prior output (https://tiptap.dev/docs/editor/extensions/custom-extensions/create-new/extension).SafeClipboardcurrently uses default priority. A subsequent transform can therefore reintroduce resource-bearing or executable markup before ProseMirror parses it, contradicting the stated complete pre-parse trust boundary for modular composition. Set and document an explicit lowest-practical sanitizer priority (or otherwise make it the final transform), and add an integration test with competing transforms proving final pre-parse output is re-sanitized. Clearly document the residual boundary for a hostile host that deliberately installs a later transform. -
Browser-realistic assurance is missing for a bespoke sanitizer: current acceptance uses jsdom only, while the implementation depends on HTML fragment parsing, CSS declaration exposure, inertness, and serialization behavior. The first exact-head failure already demonstrated a jsdom/CSSOM mismatch for
mso-hide. Add deterministic cross-engine browser tests (Chromium, Firefox, and WebKit where supported) for the security corpus and output parity, or explicitly narrow the conformance claim and record the residual acquisition risk. OWASP currently recommends a maintained HTML sanitizer such as DOMPurify and regular patching; if the no-runtime-dependency bespoke reconstruction remains the decision, the doctoring record must explain the compensating fuzz/differential-testing and vulnerability-response obligations rather than implying OWASP directly endorses this implementation.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Correction to my review 4865041574: withdraw and supersede finding 1 about the W3C publication date. Direct verification of the official dated W3C publication confirms that https://www.w3.org/TR/2026/WD-clipboard-apis-20260624/ is the genuine Clipboard API and Events — W3C Working Draft, 24 June 2026. The existing doctoring citation is therefore valid on that basis and does not require replacement. I regret the earlier false finding, which resulted from relying on a stale publication-history search result rather than opening the official dated document.
The other findings in that review remain open unless subsequent commits address them: deterministic final-transform ordering under TipTap extension composition, and browser-realistic cross-engine assurance/explicit residual-risk documentation for the bespoke sanitizer.
The earlier hostile-config integration finding is now addressed at this exact head: buildExtensions() preserves the original ClipboardConfig object without dereferencing it, and validation remains deferred to paste time. This acknowledgement does not mark any unrelated finding resolved.
|
Exact-current-head review request for CI This cycle added a test-first fail-closed boundary for native @coderabbitai review A qualifying non-author independent approval and every branch-protection gate remain mandatory. Do not merge, update the branch, alter reviewer identities, or treat queued/pending/rate-limited evidence as success. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/inkspan'
pr='65'
base='66cf4b9e46b5cbbc936ba470da5d1e96ca1649af'
head='75e96ebc2a853f9315eb166b8eb98264109b238e'
printf '%s\n' '== Revision verification =='
printf 'checked-out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
printf 'base='; git rev-parse "$base"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=yes'
printf '%s\n' '== Pull request and exact-head checks =='
gh pr view "$pr" --repo "$repo" \
--json headRefOid,baseRefOid,state,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup,reviews \
--jq '{headRefOid,baseRefOid,state,isDraft,mergeStateStatus,reviewDecision,checks:[.statusCheckRollup[]|select(.name != null)|{name,status,conclusion,headSha}],reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}]}'
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
--jq '[.check_runs[]|{name,status,conclusion,head_sha}]'
printf '%s\n' '== Unresolved review threads =='
gh api graphql \
-F owner='ContextualWisdomLab' -F name='inkspan' -F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) { nodes { isResolved } }
}
}
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)] | length'
printf '%s\n' '== Diff inventory and hygiene =='
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git diff --check "$base" "$head" || true
printf '%s\n' '== Changed TypeScript outlines =='
while IFS= read -r f; do
case "$f" in
*.ts|*.tsx)
printf '\n--- %s ---\n' "$f"
ast-grep outline "$f" --items all --type function,class,interface,type 2>/dev/null || true
;;
esac
done < <(git diff --name-only "$base" "$head")
printf '%s\n' '== Focused clipboard implementation and repair diff =='
ast-grep outline src/extensions/SafeClipboard.ts --items all || true
git diff --unified=100 "$base" "$head" -- \
src/extensions/SafeClipboard.ts \
src/extensions/SafeClipboard.test.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.coverageBranches.test.ts \
src/extensions/SafeClipboard.ambientDom.test.ts \
docs/clipboard-security.md \
docs/doctoring/safe-rich-clipboard.md \
CHANGELOG.md
printf '%s\n' '== Relevant sanitizer references and test declarations =='
rg -n -C 8 --glob '*.{ts,tsx,md}' \
'(progress|meter|noframes|noembed|mso-hide|title|details|dialog|transformPastedHTML|onClipboardError)' \
src docs CHANGELOG.md || trueLength of output: 12988 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='66cf4b9e46b5cbbc936ba470da5d1e96ca1649af'
head='75e96ebc2a853f9315eb166b8eb98264109b238e'
printf '%s\n' '== SafeClipboard structure =='
ast-grep outline src/extensions/SafeClipboard.ts --items all || true
printf '%s\n' '== SafeClipboard implementation =='
sed -n '1,240p' src/extensions/SafeClipboard.ts
sed -n '241,480p' src/extensions/SafeClipboard.ts
sed -n '481,760p' src/extensions/SafeClipboard.ts
printf '%s\n' '== New native-widget and obsolete-fallback repair =='
git diff --unified=100 "$head^" "$head" -- \
src/extensions/SafeClipboard.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.test.ts \
docs/clipboard-security.md \
docs/doctoring/native-widget-fallback-content.md \
docs/doctoring/safe-rich-clipboard.md \
CHANGELOG.md
printf '%s\n' '== Focused sanitizer tests =='
rg -n -C 20 --glob '*.{test,spec}.{ts,tsx}' \
'(progress|meter|noframes|noembed|mso-hide|title|details|dialog|node limit|depth limit|byte limit)' \
src/extensions
printf '%s\n' '== Editor integration and extension assembly =='
cat -n src/extensions/kit.ts
cat -n src/components/CwlEditor.tsx
cat -n src/collaboration/CollaborativeCwlEditor.tsx
cat -n src/index.ts
cat -n src/types.ts
printf '%s\n' '== Clipboard integration test declarations =='
rg -n -C 10 --glob '*.{test,spec}.{ts,tsx}' \
'(SafeClipboard|transformPastedHTML|onClipboardError|clipboard)' \
src/components/CwlEditor.clipboard.test.tsx \
src/collaboration/CollaborativeCwlEditor.clipboard.test.tsx \
src/extensions/kit.test.ts
printf '%s\n' '== Full-diff safety checks =='
git diff --unified=0 "$base" "$head" | rg -n -i \
'^\+.*(-----BEGIN (RSA |EC |OPENSSH )?PRIVATE KEY-----|api[_-]?key\s*[:=]|secret\s*[:=]|password\s*[:=]|authorization:\s*bearer)' \
|| true
git diff --summary "$base" "$head"Length of output: 132491
I found no actionable current-head source-level finding. The review verified these items:
I cannot approve this head yet. The exact-head A new exact-head verification is required after You are interacting with an AI system. |
|
@coderabbitai review Please review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/inkspan'
pr='65'
base='66cf4b9e46b5cbbc936ba470da5d1e96ca1649af'
head='75e96ebc2a853f9315eb166b8eb98264109b238e'
printf '%s\n' '== Revision and pull-request state =='
printf 'checked-out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
printf 'base='; git rev-parse "$base"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=yes'
gh pr view "$pr" --repo "$repo" \
--json headRefOid,baseRefOid,state,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid,baseRefOid,state,isDraft,mergeStateStatus,reviewDecision,checks:[.statusCheckRollup[]|select(.name != null)|{name,status,conclusion,headSha}]}'
printf '%s\n' '== Exact-head check runs =='
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
--jq '[.check_runs[]|{name,status,conclusion,head_sha}]'
printf '%s\n' '== Unresolved inline review threads =='
gh api graphql \
-F owner='ContextualWisdomLab' -F name='inkspan' -F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) { nodes { isResolved } }
}
}
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)] | length'
printf '%s\n' '== Full diff inventory and hygiene =='
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git diff --check "$base" "$head" || true
printf '%s\n' '== Changed TypeScript structure =='
while IFS= read -r f; do
case "$f" in
*.ts|*.tsx)
printf '\n--- %s ---\n' "$f"
ast-grep outline "$f" --items all --type function,class,interface,type 2>/dev/null || true
;;
esac
done < <(git diff --name-only "$base" "$head")
printf '%s\n' '== Sanitizer and editor implementation =='
for f in \
src/extensions/SafeClipboard.ts \
src/extensions/SafeLink.ts \
src/extensions/kit.ts \
src/components/CwlEditor.tsx \
src/collaboration/CollaborativeCwlEditor.tsx \
src/index.ts \
src/types.ts; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f"
fi
done
printf '%s\n' '== Sanitizer and integration tests =='
for f in \
src/extensions/SafeClipboard.test.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.coverageBranches.test.ts \
src/extensions/SafeClipboard.ambientDom.test.ts \
src/components/CwlEditor.clipboard.test.tsx \
src/collaboration/CollaborativeCwlEditor.clipboard.test.tsx \
src/extensions/kit.test.ts; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f"
fi
done
printf '%s\n' '== Relevant direct references =='
rg -n -C 6 --glob '*.{ts,tsx}' \
'(SafeClipboard|transformPastedHTML|onClipboardError|mso-hide|progress|meter|noframes|noembed|title|details|dialog)' \
src || trueLength of output: 12754 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='66cf4b9e46b5cbbc936ba470da5d1e96ca1649af'
head='75e96ebc2a853f9315eb166b8eb98264109b238e'
printf '%s\n' '== SafeClipboard structure and implementation =='
ast-grep outline src/extensions/SafeClipboard.ts --items all || true
cat -n src/extensions/SafeClipboard.ts
printf '%s\n' '== SafeClipboard tests =='
for f in \
src/extensions/SafeClipboard.test.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.coverageBranches.test.ts \
src/extensions/SafeClipboard.ambientDom.test.ts; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '== Extension assembly and editor integration =='
for f in \
src/extensions/SafeLink.ts \
src/extensions/kit.ts \
src/components/CwlEditor.tsx \
src/collaboration/CollaborativeCwlEditor.tsx \
src/index.ts \
src/types.ts; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '== Integration tests =='
for f in \
src/extensions/kit.test.ts \
src/components/CwlEditor.clipboard.test.tsx \
src/collaboration/CollaborativeCwlEditor.clipboard.test.tsx; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '== Runtime source diff =='
git diff --unified=100 "$base" "$head" -- \
src/extensions/SafeClipboard.ts \
src/extensions/SafeLink.ts \
src/extensions/kit.ts \
src/components/CwlEditor.tsx \
src/collaboration/CollaborativeCwlEditor.tsx \
src/index.ts \
src/types.ts
printf '%s\n' '== All transform ordering and sanitizer configuration sites =='
rg -n -C 8 --glob '*.{ts,tsx}' \
'(transformPastedHTML|SafeClipboard\.configure|SafeClipboard|onClipboardError|priority\s*:)' \
srcLength of output: 240916
I found no actionable current-head source-level finding. The review verified the bounded reconstruction allowlist, dropped-subtree handling, The requested commit remains the current pull-request head. The pull request has zero unresolved inline threads. I cannot issue approval for this head yet. The exact-head
|
|
@opencode-agent review Perform read-only independent review of exact current head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/extensions/SafeClipboard.ts (1)
534-548: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value불안전 링크 분기를 단순화하세요.
539-541행의
childParent = frame.outputParent재대입은 무연산입니다.childParent는 535행에서 이미 같은 값입니다. 또한 같은isSafeLinkHref검사가copyAllowedAttributes(401행)에서 다시 실행됩니다. 조건을 한 번만 계산하면 두 지점의 정책이 갈라질 위험이 사라집니다.♻️ 제안 리팩터
const outputName = normalizedOutputElement(sourceName); + const unwrapUnsafeLink = + outputName === 'a' && + !isSafeLinkHref(sourceElement.getAttribute('href')); let childParent = frame.outputParent; - if (outputName !== null) { - if ( - outputName === 'a' && - !isSafeLinkHref(sourceElement.getAttribute('href')) - ) { - childParent = frame.outputParent; - } else { - const outputElement = inertDocument.createElement(outputName); - copyAllowedAttributes(sourceElement, outputElement); - frame.outputParent.appendChild(outputElement); - childParent = outputElement; - } + if (outputName !== null && !unwrapUnsafeLink) { + const outputElement = inertDocument.createElement(outputName); + copyAllowedAttributes(sourceElement, outputElement); + frame.outputParent.appendChild(outputElement); + childParent = outputElement; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/extensions/SafeClipboard.ts` around lines 534 - 548, In the output-element handling around normalizedOutputElement, remove the redundant unsafe-link branch that reassigns childParent to frame.outputParent, and compute the isSafeLinkHref result once for reuse with copyAllowedAttributes. Preserve the existing behavior of skipping unsafe anchor creation while ensuring the same safety decision is used consistently.src/types.ts (1)
272-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
clipboard의 적용 시점을 주석에 명시하세요.
clipboard정책은 편집기 생성 시buildExtensions()에 전달됩니다. 편집기 생성 후 새clipboard객체로 교체해도 기존 확장의 정책은 변경되지 않습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types.ts` around lines 272 - 281, Update the documentation comment for the clipboard property in the editor configuration type to state that the policy is applied when the editor is created through buildExtensions() and that replacing the clipboard object afterward does not update existing extensions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/doctoring/closed-interactive-content.md`:
- Around line 99-100: Update the date metadata in
docs/doctoring/closed-interactive-content.md lines 99-100 by changing or
removing “Retrieved August 6, 2026” so it is not later than the August 5, 2026
review date; also update “Decision date: 2026-08-06” in
docs/doctoring/native-widget-fallback-content.md lines 3-4 to the actual
decision date of 2026-08-05.
In `@docs/doctoring/safe-rich-clipboard.md`:
- Around line 127-141: Update the “Dropped subtrees and privacy boundary”
section to include or clearly cross-reference the established rules for removing
progress, meter, noframes, and noembed elements; handling closed details and
dialog elements; and parsing CSS escapes and EOF-terminated comments in Office
hidden-content detection. Keep the existing removal and mso-hide behavior intact
while synchronizing this document with the related doctoring records.
- Around line 162-165: Update the memory-behavior statement in the documentation
near “Traversal is iterative” to remove the claim that parsing allocates at most
the bounded input representation. Describe only the enforced UTF-8 input, node,
and depth limits, and acknowledge that parsed and output trees are additionally
allocated without claiming an explicit heap or DOM memory cap.
In `@docs/superpowers/plans/2026-08-05-safe-rich-clipboard.md`:
- Around line 121-134: Update the defect count in the introductory statement to
match the four items listed under the checklist, or regroup the checklist so it
contains three implementation defects and one assurance gap. Keep the
Configuration boundary, Office hidden content, Transform ordering, and Incorrect
test assertion entries accurately represented for review traceability.
In `@src/extensions/SafeClipboard.ts`:
- Around line 10-11: Validate DEFAULT_CLIPBOARD_MAX_NODES against the
repository’s representative Word HTML fixture by measuring its traversed node
count. If the fixture exceeds the default, either raise the default to cover it
or update transformPastedHTML to provide an explicit plain-text fallback while
preserving the existing behavior for smaller inputs.
---
Nitpick comments:
In `@src/extensions/SafeClipboard.ts`:
- Around line 534-548: In the output-element handling around
normalizedOutputElement, remove the redundant unsafe-link branch that reassigns
childParent to frame.outputParent, and compute the isSafeLinkHref result once
for reuse with copyAllowedAttributes. Preserve the existing behavior of skipping
unsafe anchor creation while ensuring the same safety decision is used
consistently.
In `@src/types.ts`:
- Around line 272-281: Update the documentation comment for the clipboard
property in the editor configuration type to state that the policy is applied
when the editor is created through buildExtensions() and that replacing the
clipboard object afterward does not update existing extensions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 14587e8c-8214-472e-a0e6-c65e5fd56324
📒 Files selected for processing (26)
.github/workflows/ci.ymlARCHITECTURE.mdCHANGELOG.mddocs/clipboard-security.mddocs/doctoring/closed-interactive-content.mddocs/doctoring/css-escaped-office-hidden-content.mddocs/doctoring/native-widget-fallback-content.mddocs/doctoring/safe-rich-clipboard.mddocs/superpowers/plans/2026-08-05-safe-rich-clipboard.mddocs/superpowers/specs/2026-08-05-safe-rich-clipboard-design.mdsrc/clipboardDocumentation.test.tssrc/collaboration/CollaborativeCwlEditor.clipboard.test.tsxsrc/collaboration/CollaborativeCwlEditor.tsxsrc/components/CwlEditor.clipboard.test.tsxsrc/components/CwlEditor.tsxsrc/extensions/SafeClipboard.ambientDom.test.tssrc/extensions/SafeClipboard.coverageBranches.test.tssrc/extensions/SafeClipboard.datalistRegression.test.tssrc/extensions/SafeClipboard.securityRegression.test.tssrc/extensions/SafeClipboard.test.tssrc/extensions/SafeClipboard.tssrc/extensions/kit.test.tssrc/extensions/kit.tssrc/index.tssrc/types.tssrc/workflowExactHead.test.ts
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/inkspan'
pr='65'
head='95dc3eb8a7d40f049859d94a8019c594607db612'
printf '%s\n' '== Requested revision and pull request state =='
printf 'checked-out='; git rev-parse HEAD
printf 'requested='; git rev-parse "$head"
gh pr view "$pr" --repo "$repo" \
--json headRefOid,baseRefOid,state,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid,baseRefOid,state,isDraft,mergeStateStatus,reviewDecision,checks:[.statusCheckRollup[]|select(.name != null)|{name,status,conclusion,headSha}]}'
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
printf 'base=%s\n' "$base"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=yes'
printf '%s\n' '== Exact-head check runs and unresolved threads =='
gh api "repos/$repo/commits/$head/check-runs?per_page=100" \
--jq '[.check_runs[]|{name,status,conclusion,head_sha}]'
gh api graphql \
-F owner='ContextualWisdomLab' -F name='inkspan' -F number="$pr" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) { nodes { isResolved } }
}
}
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)] | length'
printf '%s\n' '== Complete current-head diff inventory =='
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git diff --check "$base" "$head" || true
printf '%s\n' '== Changed TypeScript structure =='
while IFS= read -r f; do
case "$f" in
*.ts|*.tsx)
printf '\n--- %s ---\n' "$f"
ast-grep outline "$f" --items all --type function,class,interface,type 2>/dev/null || true
;;
esac
done < <(git diff --name-only "$base" "$head")
printf '%s\n' '== Sanitizer, extension assembly, and editor surfaces =='
for f in \
src/extensions/SafeClipboard.ts \
src/extensions/SafeLink.ts \
src/extensions/kit.ts \
src/components/CwlEditor.tsx \
src/collaboration/CollaborativeCwlEditor.tsx \
src/index.ts \
src/types.ts; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f"
fi
done
printf '%s\n' '== Sanitizer and integration tests =='
for f in \
src/extensions/SafeClipboard.test.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.coverageBranches.test.ts \
src/extensions/SafeClipboard.ambientDom.test.ts \
src/extensions/SafeClipboard.datalistRegression.test.ts \
src/components/CwlEditor.clipboard.test.tsx \
src/collaboration/CollaborativeCwlEditor.clipboard.test.tsx \
src/extensions/kit.test.ts; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f"
fi
done
printf '%s\n' '== Direct boundary, transform, and callback references =='
rg -n -C 6 --glob '*.{ts,tsx}' \
'(SafeClipboard|transformPastedHTML|onClipboardError|mso-hide|progress|meter|noframes|noembed|datalist|title|details|dialog|priority\s*:)' \
src || trueLength of output: 13690 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='66cf4b9e46b5cbbc936ba470da5d1e96ca1649af'
prior='75e96ebc2a853f9315eb166b8eb98264109b238e'
head='95dc3eb8a7d40f049859d94a8019c594607db612'
printf '%s\n' '== Incremental changes after the prior reviewed revision =='
git merge-base --is-ancestor "$prior" "$head" && echo 'prior-is-ancestor=yes' || echo 'prior-is-ancestor=no'
git diff --stat "$prior" "$head"
git diff --name-status "$prior" "$head"
git diff --unified=100 "$prior" "$head" -- \
src/extensions/SafeClipboard.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.datalistRegression.test.ts \
src/extensions/SafeClipboard.wordCapacity.test.ts \
src/datalistDocumentation.test.ts \
src/clipboardDocumentation.test.ts \
docs/clipboard-security.md \
docs/doctoring/datalist-hidden-suggestion-content.md \
docs/doctoring/visibility-collapse-hidden-content.md \
CHANGELOG.md
printf '%s\n' '== SafeClipboard structure and implementation =='
ast-grep outline src/extensions/SafeClipboard.ts --items all || true
cat -n src/extensions/SafeClipboard.ts
printf '%s\n' '== Direct sanitizer visibility and traversal paths =='
rg -n -C 12 --glob '*.{ts,tsx}' \
'(visibility|collapse|hidden|datalist|option|DROPPED_SUBTREES|isElementHidden|sanitizeRichClipboardHtml|walk|nodeCount|maxDepth)' \
src/extensions/SafeClipboard.ts src/extensions/SafeClipboard*.test.ts
printf '%s\n' '== Focused sanitizer tests =='
for f in \
src/extensions/SafeClipboard.test.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.coverageBranches.test.ts \
src/extensions/SafeClipboard.ambientDom.test.ts \
src/extensions/SafeClipboard.datalistRegression.test.ts \
src/extensions/SafeClipboard.wordCapacity.test.ts; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '== Extension assembly and both editor integrations =='
for f in \
src/extensions/SafeLink.ts \
src/extensions/kit.ts \
src/components/CwlEditor.tsx \
src/collaboration/CollaborativeCwlEditor.tsx \
src/types.ts \
src/index.ts; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '== Editor and extension integration tests =='
for f in \
src/extensions/kit.test.ts \
src/components/CwlEditor.clipboard.test.tsx \
src/collaboration/CollaborativeCwlEditor.clipboard.test.tsx; do
printf '\n--- %s ---\n' "$f"
cat -n "$f"
done
printf '%s\n' '== All extension transform implementations and ordering sites =='
rg -n -C 8 --glob '*.{ts,tsx}' \
'(transformPastedHTML|priority\s*:|buildExtensions\s*\(|SafeClipboard\.configure)' \
srcLength of output: 120853 🏁 Script executed: #!/bin/bash
set -euo pipefail
prior='75e96ebc2a853f9315eb166b8eb98264109b238e'
head='95dc3eb8a7d40f049859d94a8019c594607db612'
printf '%s\n' '== Incremental runtime and regression-test diff =='
git diff --unified=40 "$prior" "$head" -- \
src/extensions/SafeClipboard.ts \
src/extensions/SafeClipboard.securityRegression.test.ts \
src/extensions/SafeClipboard.datalistRegression.test.ts \
src/extensions/SafeClipboard.wordCapacity.test.ts
printf '%s\n' '== SafeClipboard implementation: declarations, limits, and style parsing =='
sed -n '1,230p' src/extensions/SafeClipboard.ts
printf '%s\n' '== SafeClipboard implementation: visibility and reconstruction =='
sed -n '231,460p' src/extensions/SafeClipboard.ts
printf '%s\n' '== SafeClipboard implementation: traversal and TipTap boundary =='
sed -n '461,660p' src/extensions/SafeClipboard.ts
printf '%s\n' '== Focused visibility regression tests =='
sed -n '1,230p' src/extensions/SafeClipboard.securityRegression.test.ts
cat -n src/extensions/SafeClipboard.datalistRegression.test.ts
cat -n src/extensions/SafeClipboard.wordCapacity.test.tsLength of output: 54175
I found no actionable current-head source-level finding. The review verified these items:
I cannot approve this head yet. The exact-head A new exact-head status verification is required after You are interacting with an AI system. |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head security review for 95dc3eb8a7d40f049859d94a8019c594607db612 found one valid hidden-content gap. The finding is recorded now but must be repaired only after prerequisite PR #64 reaches main, so the SafeClipboard branch remains single-writer and is reconciled once against the authoritative architecture/changelog base. Do not resolve the thread before a test-first post-#64 fix and complete exact-head revalidation.
| } | ||
| const style = (element as HTMLElement).style; | ||
| const visibility = style.visibility.trim().toLowerCase(); | ||
| return ( |
There was a problem hiding this comment.
Valid hidden-content boundary gap: content-visibility: hidden is not recognized here, so its source-only descendants are unwrapped into ordinary editor prose after the style attribute is discarded. CSS Containment Module Level 2 specifies that content-visibility: hidden skips the element’s contents and excludes them from user-agent features similarly to display: none (W3C, CSS Containment Module Level 2, §4). This can expose text that was neither rendered nor available to accessibility/find/selection surfaces in the source.
Preserve dependency order: do not write this branch until PR #64 is integrated. On the exact post-#64 base, add the failing regression first, covering ordinary/case/whitespace/terminal-!important, CSS-comment and CSS-escaped property/value forms, plus false positives (visible, auto, hiddenly, prefixed property names). Then extend the bounded raw-style hidden-declaration path so the complete subtree is dropped without relying on engine-specific CSSOM exposure. Update operator guidance, APA 7 doctoring, documentation contracts, and CHANGELOG.md; restore 100% statement/branch/function/line coverage and every exact-head gate before resolving this thread.
1 participant
Purpose
Add a bounded, fail-closed rich-HTML paste boundary for Microsoft Word, Google Docs, email, and web content before ProseMirror parses or inserts it.
Current implementation
titlesubtrees, forms, embedded content, SVG/MathML, media, hidden subtrees, comments, remote/local images, event handlers, IDs, classes, proprietary attributes, and arbitrary styles;display: none,visibility: hidden,visibility: collapse,hidden,aria-hidden="true", and exact Officemso-hide: allas complete hidden-subtree boundaries;details, drop closeddialogsubtrees, and sanitize rendered contents of open variants;progress,meter,datalist,noframes, andnoembedsubtrees so native-widget, hidden-suggestion, or obsolete fallback text cannot become ordinary editor prose after wrapper removal;mso-hide: allmatching while retaining malformed and look-alike declarations as visible text;Test-first hardening
dac8fef84baa313d9cada2e1c3c2bbf658b7675d/ GREENe6d51c9b3fdaffc707d11649438aa9dcdd9f932e: consume a final CSS comment through EOF somso-hide: all/*cannot surface hidden text;c11617099f03cf691dfa5d7c116650faf5ad2b40/ GREENd9fcc0f1da56e702b4efcb1e679cba70f2fb5efb: discardtitlemetadata subtrees instead of unwrapping metadata text into the editor;eebf18a623b7702e55995c4d406fe07203edfd39/ GREENe84261e74dcb07ed6afec9a934adcf5b8b1e41e3: prevent closed disclosure and dialog source-only text from becoming visible editor content while retaining rendered summary/open content;6e6d48ff8a377bab875d6a520580e65d3489f78e/ GREEN01683cc89faadfcacf05f358e34b6a1812e61e77: preventprogress,meter,noframes, andnoembedfallback descendants from surfacing after wrapper removal;a48d81eac541effea5d7742a47c0ca6bd01d04ed/ GREENec5d602bcf4f8e82fcc3136a82e354816913f6df: prevent hiddendatalistsuggestions and down-level fallback descendants from surfacing as ordinary editor prose;6dadce2459594d1721468086c8c60b8605ce0f52/ GREEN00d163eecffafa0b4c6f108f45bc4af39273c41e: prove one safe anchorhrefis read and classified exactly once and reuse the validated value without a second policy evaluation;77ea951c54230f896788157913a924186ae487f7/ GREEN503ba7f74001c2bfc05bee76d154fc7b162200f3: preventvisibility: collapsetable rows, cells, and ordinary subtrees from exposing non-rendered source text while retaining visible siblings; andCurrent post-prerequisite finding
Exact-head review of
95dc3eb8a7d40f049859d94a8019c594607db612identified one valid hidden-content gap:content-visibility: hiddenis not recognized, so source-only descendants can be unwrapped into ordinary editor prose after the sourcestyleattribute is discarded.The current inline thread remains intentionally unresolved. To prevent competing branch writers and preserve dependency order, this branch must not be modified until PR #64 reaches
main. On the exact integrated base, the repair must be test-first and cover ordinary, case, whitespace, terminal!important, CSS-comment, and CSS-escaped forms plus false positives such asvisible,auto,hiddenly, and prefixed property names. The implementation must use the bounded raw-style hidden-declaration boundary rather than depend on engine-specific CSSOM exposure, then update operator guidance, APA 7 doctoring, documentation contracts, andCHANGELOG.md.Review and documentation reconciliation
docs/doctoring/visibility-collapse-hidden-content.mdwith W3C CSS visibility/collapse rationale, APA 7th references, test evidence, rollback, and compatibility boundaries;content-visibility: hiddenfinding as unresolved until the post-docs: define Inkspan modular acquisition architecture #64 integrated fix passes every gate.Unresolved inline review threads: 1.
Exact-head verification
Verified code head:
95dc3eb8a7d40f049859d94a8019c594607db612.31058980885: passed TypeScript typecheck, 100% production statement/branch/function/line coverage, deterministic builds, packed ESM/CommonJS/strict-TypeScript consumers, SSR-safe imports, demo build, and Office Python 3.11/3.14 dependency/docstring/100%-branch-coverage/wheel/package gates;31058980977: passed;31058980840: passed; andThese successful code-head checks predate the newly recorded valid review finding and do not establish merge readiness. The predecessor RED head
77ea951c54230f896788157913a924186ae487f7failed CI run31058675380as intended before the production repair; its successful security runs are retained only as test-first history and are not used as merge evidence.Standards and assurance boundary
The design and doctoring records cite the official W3C Clipboard API and Events Working Draft, WHATWG HTML parsing/rendering semantics, W3C CSS visibility and table-collapse semantics, W3C CSS Syntax Module Level 3 comment consumption, OWASP guidance, and TipTap/ProseMirror paste-transform contracts in APA 7th style. The post-prerequisite repair must add the W3C CSS Containment Module Level 2
content-visibilitydecision and its claim boundary. Current jsdom evidence is not represented as cross-engine browser conformance; a dependency-locked Chromium, Firefox, and WebKit differential corpus remains a publication gate for 0.6.0.Dependency and integration order
PR #64 is the authoritative standalone and modular acquisition-architecture prerequisite. Both branches currently add
ARCHITECTURE.mdand modifyCHANGELOG.mdfrom the same base, so merging or writing them in parallel would create competing ownership and an avoidable integration conflict.Required order:
mainhead, preserve docs: define Inkspan modular acquisition architecture #64's authoritative architecture contract, and reconcile only the SafeClipboard-specific architecture and changelog additions;content-visibility: hiddenregression before the implementation repair, update authoritative documentation and doctoring, and restore 100% statement/branch/function/line coverage;This PR remains Draft until PR #64 is integrated, the valid hidden-content finding is addressed, and the exact post-integration head is revalidated. No parallel repair agent or workflow may write either branch.
Remaining merge gates
Do not merge until:
content-visibility: hiddenfinding is fixed test-first and its thread is resolved only after exact-head verification;The current-head CodeRabbit read-only comment found no actionable source-level issue, but it is not a formal approval and does not supersede the separately identified unresolved hidden-content finding. The default paste behavior change targets 0.6.0 only through a separate verified release PR.